-
Couldn't load subscription status.
- Fork 2
Add a deprecated_member() function to fix typing
#98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a new deprecated_member() function to improve typing when creating deprecated enum members. The function provides a type-safe wrapper around the existing DeprecatedMember class.
- Adds a generic
deprecated_member()function that returns the correct type for mypy - Updates all existing usage of
DeprecatedMemberto use the new function - Updates documentation and examples to reflect the new preferred approach
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/frequenz/core/enum.py | Adds the new deprecated_member() function and updates documentation |
| tests/test_enum.py | Updates tests to use new function and adds comprehensive test coverage |
| README.md | Updates example to demonstrate the new function usage |
| RELEASE_NOTES.md | Documents the new feature and upgrade guidance |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
1d48054 to
5345a23
Compare
| """ | ||
| return cast(ValueT, DeprecatedMember(value, message)) | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So will you deprecate direct usage of DeprecatedMember then? 😆
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about it. But I think it is too meta 😆
But when we go for 2.0.0 I think I would make it private, we really don't need to expose it at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh, so nice to have gotten issue number 100 for:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a comment to check for. And LGTM
When using the `DeprecatedMember` class directly, `mypy` will struggle because it will think the actual type of the value is `DeprecatedMember` instead of the real underlying value. To fix this we create a simple generic wrapper that will cast the `DeprecatedMember` to the type of the value. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
5345a23 to
9830fad
Compare
|
Updated. |
When using the
DeprecatedMemberclass directly,mypywill struggle because it will think the actual type of the value isDeprecatedMemberinstead of the real underlying value.To fix this we create a simple generic wrapper
deprecated_member()that will cast theDeprecatedMemberto the type of the value.